Skip to content

Conversation

@weyert
Copy link
Contributor

@weyert weyert commented Dec 9, 2018

Detects whether flow-bin is a dependency on the project, if so it will load the @babel/preset-flow-module.

What:

In babelrc.js we should check if flow-bin is a dependency and then load the @babel/preset-flow

Why:
We have copied the same away react is being detected by using ifAnyDep-utility

How:

Small change to babelrc.js

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

Detects whether `flow-bin` is a dependency on the project, if so it will load the `@babel/preset-flow`-module
Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'm in favor of this 👍 just one thing.

ifAnyDep(
['flow-bin'],
[
require.resolve('@babel/preset-flow'),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add @babel/preset-flow to the dependencies so folks don't have to install it.

@kentcdodds
Copy link
Owner

Interesting that the build is failing for not having @babel/preset-flow but I wouldn't expect that preset to be required because this project does not have a dependency on flow-bin. Seems like this may be buggy somehow...

@weyert
Copy link
Contributor Author

weyert commented Dec 9, 2018

Really strange that preset-flow is necessary, I didn't expect that. I will have a look into it

@weyert
Copy link
Contributor Author

weyert commented Dec 9, 2018

The problem should be resolved now @kentcdodds

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

{pragma: isPreact ? 'React.h' : undefined},
],
),
ifAnyDep(['flow-bin'], [optionalRequire('@babel/preset-flow')]),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be necessary. I think that we should actually include @babel/preset-flow in kcd-scripts' dependencies. (Part of the idea of this package is you generally shouldn't have to install tooling dependencies).

So we can get rid of optionalRequire.

@weyert
Copy link
Contributor Author

weyert commented Dec 10, 2018

Sorry, goofed up my git repo. Making a new repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants